Add destroy_dev() function as a counterpart to create_dev() by julianuziemblo · Pull Request #466 · phoenix-rtos/libphoenix

julianuziemblo · 2026-03-16T16:39:23Z

Description

Motivation and Context

Depends-On: phoenix-rtos-kernel:julianuziemblo/port-unregister

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)
Chore (refactoring, style fixes, git/CI config, submodule management, no code logic changes)

How Has This Been Tested?

Already covered by automatic testing.
New test added: (add PR link here).
Tested by hand on: armv7m7-imxrt117x-evk

Checklist:

My change requires a change to the documentation.
I have updated the documentation accordingly.
I have added tests to cover my changes.
All new and existing linter checks and tests passed.
My changes generate no new compilation warnings for any of the targets.

Special treatment

This PR needs additional PRs to work:
- !syscalls: add portUnregister syscall phoenix-rtos-kernel#750
I will merge this PR by myself when appropriate.

gemini-code-assist · 2026-03-16T16:39:40Z

Summary of Changes

Hello, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request enhances the system's device and port management by introducing explicit functions for unregistering devices and ports. This new functionality improves resource cleanup and system stability, particularly in dynamic environments where devices are frequently added and removed, by providing a structured way to remove device entries.

Highlights

Device Removal Functionality: Introduced a new remove_dev function to allow for the unregistration of devices from the system, enhancing device management capabilities.
Port Unregistration: Added a portUnregister function, providing a mechanism to unregister communication ports.
Robust Device Unregistration: The remove_dev implementation includes logic to handle scenarios where the device filesystem (devfs) might not be immediately available, falling back to portUnregister if necessary.

Changelog

include/posix/utils.h
- Declared the remove_dev function prototype.
include/sys/msg.h
- Declared the portUnregister function prototype.
unistd/file.c
- Implemented the remove_dev function, including logic for interacting with the device filesystem and falling back to portUnregister.

Activity

The author marked this as a "New feature" type of change.
The changes were tested by hand on armv7m7-imxrt117x-evk.
All new and existing linter checks and tests passed.
The changes generated no new compilation warnings for any targets.
This PR is noted to require additional PRs to work, specifically depending on phoenix-rtos-kernel:julianuziemblo/port-unregister.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	`/gemini review`	Performs a code review for the current pull request in its current state.
Pull Request Summary	`/gemini summary`	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	`/gemini help`	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

gemini-code-assist

Code Review

This pull request introduces a new remove_dev function and declares portUnregister for device removal. The implementation of remove_dev is mostly correct, but I've found a bug in how it handles path prefixes which could lead to incorrect behavior. I've also noted an inconsistency in error handling compared to similar functions in the same file. My review includes suggestions to address these issues.

unistd/file.c

nalajcie · 2026-03-16T17:09:09Z

I would rather see create_dev rewritten first. IMHO this function should not try to create /dev dir, also should either accept /dev/devname or devname param, not try to handle all cases. Probably some other thing should also be straightened out here.

github-actions · 2026-03-16T17:27:47Z

Unit Test Results

9 553 tests ±0 8 961 ✅ ±0 53m 10s ⏱️ +38s
591 suites ±0 592 💤 ±0
1 files ±0 0 ❌ ±0

Results for commit 2b7f7e9. ± Comparison against base commit 74852cb.

♻️ This comment has been updated with latest results.

julianuziemblo · 2026-03-16T17:28:37Z

I would rather see create_dev rewritten first. IMHO this function should not try to create /dev dir, also should either accept /dev/devname or devname param, not try to handle all cases. Probably some other thing should also be straightened out here.

I agree with your suggestions, but it seems like they should he handled in separate PR as they are big and very breaking. This PR only adds missing functionality as a basis for drivers that need it now to work without creating excessive overhead (e.g. spawning cleanup threads just to take care of deleting their own devs).

agkaminski

One nitpick, otherwise LGTM

unistd/file.c

sys/msg.c

unistd/file.c

destroy_dev() is a counterpart to create_dev() that allows for device removal without sending messages back to the calling port unlike remove()/unlink() functions YT: RTOS-1254

To make destroy_dev() resemble create_dev(), a fallback was added to unregister the port in the case we don't have a filesystem. YT: RTOS-1254

… syscalls YT: RTOS-1254

YT: RTOS-1254

gemini-code-assist bot reviewed Mar 16, 2026

View reviewed changes

unistd/file.c Outdated Show resolved Hide resolved

unistd/file.c Outdated Show resolved Hide resolved

julianuziemblo mentioned this pull request Mar 16, 2026

!syscalls: add portUnregister syscall phoenix-rtos/phoenix-rtos-kernel#750

Open

14 tasks

julianuziemblo changed the title ~~Julianuziemblo/remove dev~~ Add remove_dev() function as a counterpart to create_dev() Mar 16, 2026

julianuziemblo force-pushed the julianuziemblo/remove_dev branch from ad281e5 to 066b430 Compare March 16, 2026 16:51

julianuziemblo force-pushed the julianuziemblo/remove_dev branch from 585aecd to 066b430 Compare March 17, 2026 08:15

julianuziemblo requested review from Darchiv and agkaminski March 17, 2026 08:39

agkaminski reviewed Mar 17, 2026

View reviewed changes

unistd/file.c Outdated Show resolved Hide resolved

julianuziemblo force-pushed the julianuziemblo/remove_dev branch from 066b430 to fb718ea Compare March 17, 2026 10:54

julianuziemblo changed the title ~~Add remove_dev() function as a counterpart to create_dev()~~ Add destroy_dev() function as a counterpart to create_dev() Mar 17, 2026

julianuziemblo force-pushed the julianuziemblo/remove_dev branch from fb718ea to 417af95 Compare March 17, 2026 11:12

agkaminski reviewed Mar 17, 2026

View reviewed changes

sys/msg.c Outdated Show resolved Hide resolved

julianuziemblo force-pushed the julianuziemblo/remove_dev branch 2 times, most recently from add8987 to 94e003f Compare March 17, 2026 12:38

github-actions bot reviewed Mar 17, 2026

View reviewed changes

sys/msg.c Outdated Show resolved Hide resolved

julianuziemblo force-pushed the julianuziemblo/remove_dev branch from 94e003f to a3a1cce Compare March 17, 2026 12:38

agkaminski previously approved these changes Mar 17, 2026

View reviewed changes

julianuziemblo mentioned this pull request Mar 17, 2026

wi-fi: support STA mode phoenix-rtos/phoenix-rtos-lwip#129

Open

14 tasks

julianuziemblo dismissed agkaminski’s stale review via 694d089 March 18, 2026 10:55

julianuziemblo force-pushed the julianuziemblo/remove_dev branch from a3a1cce to 694d089 Compare March 18, 2026 10:55

github-actions bot reviewed Mar 18, 2026

View reviewed changes

unistd/file.c Outdated Show resolved Hide resolved

unistd/file.c Outdated Show resolved Hide resolved

julianuziemblo mentioned this pull request Mar 18, 2026

Implement CYW4373E USB device driver phoenix-rtos/phoenix-rtos-devices#596

Open

14 tasks

julianuziemblo force-pushed the julianuziemblo/remove_dev branch 2 times, most recently from 2e754d2 to 56d6117 Compare March 20, 2026 13:35

julianuziemblo added 2 commits March 20, 2026 15:42

unistd/file: add destroy_dev()

0ff9fe2

destroy_dev() is a counterpart to create_dev() that allows for device removal without sending messages back to the calling port unlike remove()/unlink() functions YT: RTOS-1254

unistd: add portUnregister syscall to help with destroy_dev() error path

776f1b1

To make destroy_dev() resemble create_dev(), a fallback was added to unregister the port in the case we don't have a filesystem. YT: RTOS-1254

julianuziemblo added 2 commits March 20, 2026 15:43

!sys/msg: pass string len for sys_portRegister and sys_portUnregister…

0de023b

… syscalls YT: RTOS-1254

posix/utils: include sys/types.h for oid_t type

2b7f7e9

YT: RTOS-1254

julianuziemblo force-pushed the julianuziemblo/remove_dev branch from 56d6117 to 2b7f7e9 Compare March 20, 2026 14:43

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add destroy_dev() function as a counterpart to create_dev()#466

Add destroy_dev() function as a counterpart to create_dev()#466
julianuziemblo wants to merge 4 commits intomasterfrom
julianuziemblo/remove_dev

julianuziemblo commented Mar 16, 2026 •

edited

Loading

Uh oh!

gemini-code-assist bot commented Mar 16, 2026

Uh oh!

gemini-code-assist bot left a comment

Uh oh!

Uh oh!

Uh oh!

nalajcie commented Mar 16, 2026

Uh oh!

github-actions bot commented Mar 16, 2026 •

edited

Loading

Uh oh!

julianuziemblo commented Mar 16, 2026

Uh oh!

agkaminski left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

julianuziemblo commented Mar 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Motivation and Context

Types of changes

How Has This Been Tested?

Checklist:

Special treatment

Uh oh!

gemini-code-assist bot commented Mar 16, 2026

Summary of Changes

Highlights

Footnotes

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

nalajcie commented Mar 16, 2026

Uh oh!

github-actions bot commented Mar 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Unit Test Results

Uh oh!

julianuziemblo commented Mar 16, 2026

Uh oh!

agkaminski left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

julianuziemblo commented Mar 16, 2026 •

edited

Loading

github-actions bot commented Mar 16, 2026 •

edited

Loading